Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Buildozer's venv handling. #1689

Merged
merged 2 commits into from
Sep 10, 2023
Merged

Improve Buildozer's venv handling. #1689

merged 2 commits into from
Sep 10, 2023

Conversation

Julian-O
Copy link
Contributor

  • The mechanism used to determine if it was currently running in a venv was non-standard. Used technique recommended by venv documentation.
  • Rather than call a subprocess to run venv, use the built-in venv library.
  • Rather than check if an attribute exists, define it in __init__() and see if it has changed.
  • self.venv contained a directory path, but the folder name didn't need to be stored after the method returned. A Boolean was all that was required.

Also, some trivial clean ups thrown in:

  • Comment improvements.
  • Copy command didn't need to be logged; buildops will do that.
  • Directory's existence doesn't need to be checked; buildops will do that.

* Rather than call a subprocess to run venv, use the built-in venv library.
* Rather than check if an attribute exists, define it in __init__ and see if it has changed.
* self.venv contained a folder name, but the folder name didn't need to be stored after the method returned. A boolean was all that was required.

Also, some trivial clean ups thrown in:
* Comment improvements.
* Copy command didn't need to be logged; buildops will do that.
* Directory's existence doesn't need to be checked; buildops will do that.
buildozer/__init__.py Outdated Show resolved Hide resolved
Co-authored-by: Mirko Galimberti <[email protected]>
@Julian-O
Copy link
Contributor Author

Good idea. Updated.

Copy link
Member

@misl6 misl6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you!

@misl6 misl6 merged commit 40fbf38 into kivy:master Sep 10, 2023
15 checks passed
@Julian-O Julian-O deleted the venvhandling branch September 11, 2023 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants